⚠️ Potential issue | 🟡 Minor

find_i2c always selects the first I2C controller found.

On hardware with multiple I2C buses (e.g., a bus dedicated to a display and another to sensors), the AHT10 may not be on the first controller discovered. Consider accepting an I2C bus index or identifier via ctx (already available as the callback parameter) to allow callers to specify the intended bus.

The way I2C device drivers work is that their parent should be an I2C controller.
The parent is set in the Device instance itself:

/** Represents a piece of hardware */
struct Device {
    /** The name of the device. Valid characters: a-z a-Z 0-9 - _ . */
    const char* name;

    /** The configuration data for the device's driver */
    const void* config;

    /** The parent device that this device belongs to. Can be NULL, but only the root device should have a NULL parent. */
    struct Device* parent;

    /**
     * Internal state managed by the kernel.
     * Device implementers should initialize this to NULL.
     * Do not access or modify directly; use device_* functions.
     */
    struct DeviceInternal* internal;
};

This way, the I2C (child) device driver doesn't need to know directly which bus it belongs to: it can assume that the parent is an I2C controller device.

I recently made an I2C device driver that you could look at for reference:
https://github.com/TactilityProject/Tactility/tree/main/Drivers/pi4ioe5v6408-module

(with the caveat that it is only a simple I2C device for now, this specific one should eventually emit a proper GPIO controller, but you can disregard that)

@KenVanHoeylandt, thank you for the clarification! I understand now—in the Tactility framework, I2C device drivers should rely on the parent-child relationship defined in the Device struct rather than enumerating and selecting controllers. The parent field establishes which I2C controller the device belongs to, so the device driver can simply assume its parent is the appropriate I2C controller.

This makes the architecture much cleaner and avoids the need for selection logic. I'll keep this pattern in mind for future reviews of I2C device drivers in this project.

🐰💡

Learnt from: KenVanHoeylandt
Repo: TactilityProject/TactilityApps PR: 28
File: Apps/AHT10Sensor/main/Source/aht10.c:15-19
Timestamp: 2026-02-26T17:36:39.298Z
Learning: In TactilityProject repositories, I2C device drivers should use the parent-child relationship defined in the `Device` struct. The `parent` field in the Device struct should be set to the appropriate I2C controller device. Device drivers can assume their parent is the correct I2C controller and don't need to enumerate or select from multiple controllers.

Learnt from: KenVanHoeylandt
Repo: TactilityProject/TactilityApps PR: 21
File: Apps/MystifyDemo/tactility.py:382-383
Timestamp: 2026-02-06T19:33:55.104Z
Learning: In the TactilityProject/TactilityApps repository, for any file named `tactility.py`, only report major issues or worse during code reviews. Skip minor issues, informational notes, and trivial suggestions.

#!/bin/bash
# Fetch the reference I2C device driver implementation
curl -s https://raw.githubusercontent.com/TactilityProject/Tactility/main/Drivers/pi4ioe5v6408-module/pi4ioe5v6408.c | head -100

#!/bin/bash
# Use GitHub API to explore the Drivers directory
gh api repos/TactilityProject/Tactility/contents/Drivers --jq '.[].name'

#!/bin/bash
# List contents of the pi4ioe5v6408-module directory
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module --jq '.[].name'

# Fetch the main implementation file
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/pi4ioe5v6408.c --jq -r '.content' | base64 -d

#!/bin/bash
# List contents of the source directory
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source --jq '.[].name'

# Fetch the main C file
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/pi4ioe5v6408.c --jq -r '.content' | base64 -d

#!/bin/bash
# Fetch the pi4ioe5v6408.cpp file
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/pi4ioe5v6408.cpp --jq -r '.content' | base64 -d

echo "---"
echo "Fetching module.cpp:"
echo "---"

# Also fetch module.cpp to see the structure
gh api repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/module.cpp --jq -r '.content' | base64 -d

#!/bin/bash
# Fetch raw content directly using GitHub API
curl -s -H "Accept: application/vnd.github.raw" \
  "https://api.github.com/repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/pi4ioe5v6408.cpp" 

echo ""
echo "===================="
echo "module.cpp:"
echo "===================="

curl -s -H "Accept: application/vnd.github.raw" \
  "https://api.github.com/repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/module.cpp"

#!/bin/bash
# Use gh api with authentication to fetch the file
gh api /repos/TactilityProject/Tactility/contents/Drivers/pi4ioe5v6408-module/source/pi4ioe5v6408.cpp \
  --jq '.content' | python3 -c "import sys, base64; print(base64.b64decode(sys.stdin.read()).decode())"

@KenVanHoeylandt Thank you for the reference! I can see the proper pattern now. In the pi4ioe5v6408 driver, the device uses device_get_parent(device) to get the I2C controller rather than searching for one globally. Each I2C operation then passes the parent to the i2c_controller_* functions:

auto* parent = device_get_parent(device);
return i2c_controller_register8_set(parent, GET_CONFIG(device)->address, ...);

This makes sense architecturally - the parent-child relationship in the Device structure defines which I2C bus the device belongs to, eliminating the need for global discovery.

I can see that the current AHT10 implementation uses a different approach (global search via find_i2c()), but I understand from your explanation that properly structured device drivers rely on the parent relationship instead.

Learnt from: KenVanHoeylandt
Repo: TactilityProject/TactilityApps PR: 28
File: Apps/AHT10Sensor/main/Source/aht10.c:15-19
Timestamp: 2026-02-26T17:37:36.652Z
Learning: In the TactilityProject repositories, I2C device drivers should use `device_get_parent(device)` to obtain their I2C controller, not search for controllers globally. The parent Device pointer defines which I2C bus the device belongs to. All I2C operations should pass the parent device to `i2c_controller_*` functions. See the pi4ioe5v6408 driver as a reference implementation.

Learnt from: KenVanHoeylandt
Repo: TactilityProject/TactilityApps PR: 21
File: Apps/MystifyDemo/tactility.py:382-383
Timestamp: 2026-02-06T19:33:55.104Z
Learning: In the TactilityProject/TactilityApps repository, for any file named `tactility.py`, only report major issues or worse during code reviews. Skip minor issues, informational notes, and trivial suggestions.

⚠️ Potential issue | 🟠 Major

vTaskDelay(80 ms) inside an LVGL timer callback blocks the LVGL task.

The call chain is lv_timer_create(sensor_update) → sensor_update → aht10_read → vTaskDelay(pdMS_TO_TICKS(80)). LVGL timers fire within the LVGL task; blocking that task for 80 ms every 2 seconds stalls all LVGL rendering and event processing for that window, causing periodic UI freezes.

The recommended fix is to decouple the I2C read from the LVGL thread — either dedicate a FreeRTOS task to sensor reading and share results with the LVGL thread via a mutex-protected struct, or split the read into a two-step state machine (trigger in one timer tick, read in the next after 80 ms has elapsed).

⚠️ Potential issue | 🟠 Major

Status byte (data[0]) is never checked before parsing measurement data.

Per the AHT10 datasheet, byte 0 of the response contains the status register:

• Bit 7 (busy flag) — 1 means measurement is still in progress; data in bytes 1–5 is invalid.
• Bit 3 (calibration flag) — 0 means the sensor is uncalibrated; readings are unreliable.

Skipping these checks means that if the sensor is still busy (however unlikely after 80 ms) or uncalibrated, corrupt values are silently written to *humidity and *temperature.

     if (i2c_controller_read(
             s_i2c,
             AHT10_ADDR,
             data,
             sizeof(data),
             pdMS_TO_TICKS(100)) != ERROR_NONE)
         return false;

+    // Verify sensor is not busy and is calibrated
+    if ((data[0] & 0x80) || !(data[0] & 0x08))
+        return false;
+
     uint32_t raw_h =

⚠️ Potential issue | 🟡 Minor

arc_temp missing from the null guard.

label_temp and label_hum are guarded but arc_temp is used on line 32 without a null check. While onHideApp deletes the timer before nulling the pointer (making this safe today), the guard is inconsistent and becomes a latent NPE if the cleanup order ever changes.

-    if (!label_temp || !label_hum)
+    if (!arc_temp || !label_temp || !label_hum)
         return;

⚠️ Potential issue | 🟡 Minor

Arc range 0–50°C is too narrow for the AHT10's full output range.

The AHT10 measures from −40°C to +85°C. Temperatures outside 0–50°C will be silently clamped by lv_arc_set_value, showing incorrect arc position while the label displays the true value — creating a confusing visual mismatch.

⚠️ Potential issue | 🟡 Minor

aht10_init() return value silently discarded.

If the sensor is absent or I2C setup fails, the app proceeds normally — the timer fires, aht10_read fails immediately, and the UI remains stuck on placeholder values with no indication of failure. At minimum, consider logging the error or displaying an error state in the UI.

The way I2C device drivers work is that their parent should be an I2C controller.
The parent is set in the Device instance itself:

/** Represents a piece of hardware */
struct Device {
    /** The name of the device. Valid characters: a-z a-Z 0-9 - _ . */
    const char* name;

    /** The configuration data for the device's driver */
    const void* config;

    /** The parent device that this device belongs to. Can be NULL, but only the root device should have a NULL parent. */
    struct Device* parent;

    /**
     * Internal state managed by the kernel.
     * Device implementers should initialize this to NULL.
     * Do not access or modify directly; use device_* functions.
     */
    struct DeviceInternal* internal;
};

This way, the I2C (child) device driver doesn't need to know directly which bus it belongs to: it can assume that the parent is an I2C controller device.

I recently made an I2C device driver that you could look at for reference:
https://github.com/TactilityProject/Tactility/tree/main/Drivers/pi4ioe5v6408-module

(with the caveat that it is only a simple I2C device for now, this specific one should eventually emit a proper GPIO controller, but you can disregard that)

Drivers can be used on more than 1 I2C bus, so storing the I2C device globally would break in that scenario.

If a driver needs to store data related to a device, it can do so with device_set_driver_data().

In this case, the I2C controller doesn't need to be stored, as it should be the parent device of our aht10 device. (see my other comment)

Drivers should not have an init call. Init should be part of the start/stop behaviour of the driver.

We recently removed all the symbolic links in favour of a single tactility.py in the root directory of the project, so this one can be removed.
(you probably started this app before that change happened)

This is not going to fit on small displays.